Add missing endpoints (/compatibility, /mode) to SchemaRegistryClient #2024

fangnx · 2025-08-18T20:36:26Z

What

Add the missing endpoints and update the existing ones to SR client, for /compatibility and /mode endpoints.

Checklist

Contains customer facing changes? Including API/behavior changes
Did you add sufficient unit test and/or integration test coverage for this PR?
- If not, please explain why it is not required

References

JIRA: https://confluentinc.atlassian.net/browse/DGS-21591

Test & Review

Open questions / Follow-ups

confluent-cla-assistant · 2025-08-18T20:36:37Z

🎉 All Contributor License Agreements have been signed. Ready to merge.
_{Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.}

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

MSeal · 2025-08-18T21:52:58Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

        )
+        return response['is_compatible']  # TODO: should it return entire response (including error messages)?


Maybe return a tuple [bool, Response] so it's easy to have a simple response vs a parsing the complex object? But I think this should probably return the full response by itself with a reference in the docstring to the is_compatible as a likely attribute for consumption.

My concern is this would be a breaking change, although in this case I think it might be necessary to just make the change (or add a separate method and mark this one as @deprecated): not sure how customers usually interact with test_compatibility, but intuitively I think the "messages" field for test_compatibility is important to include
cc @rayokota if you have any thoughts on this :)

MSeal · 2025-08-18T22:01:31Z

src/confluent_kafka/schema_registry/_sync/schema_registry_client.py

+            `GET Global Mode API Reference <https://docs.confluent.io/current/schema-registry/develop/api.html#get--mode>`_
+        """  # noqa: E501
+        result = self._rest_client.get('mode')
+        return result['mode']


Consider a debug log here with the result if you're not returning the full payload so the contents aren't lost (or maybe generally have a logging debug level option for all rest requests made to SR service)

In this case, mode is the only field in the JSON returned from this API

Copilot

Pull Request Overview

This PR adds missing endpoints for /compatibility and /mode operations to the Schema Registry Client, expanding the client's API coverage to match the complete Schema Registry REST API.

Key changes include:

Added new mode management endpoints for global and subject-level mode operations
Enhanced compatibility testing with new all-versions endpoint and query parameters
Added configuration deletion endpoint and contexts listing endpoint

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
tests/schema_registry/conftest.py	Updated mock server to support new endpoints with proper regex patterns and callback handlers
tests/schema_registry/_sync/test_api_client.py	Added comprehensive test coverage for all new sync client methods
tests/schema_registry/_async/test_api_client.py	Added comprehensive test coverage for all new async client methods
src/confluent_kafka/schema_registry/_sync/schema_registry_client.py	Implemented new endpoint methods and enhanced existing compatibility testing
src/confluent_kafka/schema_registry/_async/schema_registry_client.py	Implemented new endpoint methods and enhanced existing compatibility testing

Comments suppressed due to low confidence (1)

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-21T20:36:10Z

tests/schema_registry/conftest.py

@@ -97,10 +97,12 @@
 +--------+-------------------------------------------------+-------+------------------------------+
 | DELETE | /subjects/notfound/versions/422                 | 42202 | Invalid version              |
 +--------+-------------------------------------------------+-------+------------------------------+
-| GET    | /config/notconfig                               | 40401 | Subject not found            |
+| GET    | /config/notfound                                | 40401 | Subject not found            |


[nitpick] The table entry for GET /config/notfound shows error code 40401 but the actual mock implementation on line 234 returns the same error code. Consider clarifying if this path should represent a different test case since 'notfound' is used as a trigger for error scenarios.

Suggested change

| GET | /config/notfound | 40401 | Subject not found |

| GET | /config/notfound | 40401 | Subject not found (uses 'notfound' trigger) |

Copilot · 2025-08-21T20:36:10Z

tests/schema_registry/conftest.py

    COUNTER['GET'][request.url.path] += 1
-    return Response(200, json=json.loads(_load_avsc(SCHEMA)))
+    return Response(200, json=[1, 2])


 def get_subject_version_referenced_by_callback(request, route):


This function is defined twice in the same file (lines 410 and 415), which will cause the second definition to override the first. This could lead to unexpected behavior in tests.

Copilot · 2025-08-21T20:36:10Z

tests/schema_registry/conftest.py

+def put_global_mode_callback(request, route):
+    COUNTER['PUT'][request.url.path] += 1
+    body = json.loads(request.content.decode('utf-8'))
+    print(body)


Debug print statement should be removed from production code. Consider using logging instead if debugging output is needed.

Suggested change

print(body)

logger.debug(body)

Copilot · 2025-08-21T20:36:11Z

src/confluent_kafka/schema_registry/_sync/schema_registry_client.py

@@ -889,6 +888,13 @@ def get_subjects(
        """
        Lists all subjects registered with the Schema Registry.

+        Args:


Duplicate Args section in docstring. The Args section appears twice (lines 891-897 and 898-904), which creates confusing documentation.

Copilot · 2025-08-21T20:36:11Z

src/confluent_kafka/schema_registry/_sync/schema_registry_client.py

+    def test_compatibility_all_versions(
+        self, subject_name: str, schema: 'Schema',
+        normalize: bool = False, verbose: bool = False
+    ) -> bool:


[nitpick] TODO comment suggests uncertainty about the API design. Consider resolving this before merging or creating a follow-up ticket to address whether detailed error messages should be returned.

Copilot · 2025-08-21T20:36:13Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

@@ -889,6 +888,13 @@ async def get_subjects(
        """
        Lists all subjects registered with the Schema Registry.

+        Args:


Duplicate Args section in docstring. The Args section appears twice (lines 891-897 and 898-904), which creates confusing documentation.

Copilot · 2025-08-21T20:36:13Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

        )
+        return response['is_compatible']  # TODO: should it return entire response (including error messages)?


[nitpick] TODO comment suggests uncertainty about the API design. Consider resolving this before merging or creating a follow-up ticket to address whether detailed error messages should be returned.

Suggested change

return response['is_compatible'] # TODO: should it return entire response (including error messages)?

return response

Copilot · 2025-08-21T20:36:13Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

+            subject_name (str): Subject of the schema versions against which compatibility is to be tested.
+            schema (Schema): Schema instance.
+            normalize (bool): Whether to normalize the input schema.
+            verbose (bool): Wehther to return detailed error messages.


Typo in parameter documentation: 'Wehther' should be 'Whether'.

Suggested change

verbose (bool): Wehther to return detailed error messages.

verbose (bool): Whether to return detailed error messages.

Copilot · 2025-08-21T20:36:13Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

+            ),
+            body=request,
+        )
+        return response['is_compatible']  # TODO: should it return entire response (including error messages)?


[nitpick] Duplicate TODO comment with same uncertainty about API design. This appears in multiple methods suggesting a broader design decision is needed.

Suggested change

return response['is_compatible'] # TODO: should it return entire response (including error messages)?

return response

Copilot · 2025-08-21T20:36:14Z

src/confluent_kafka/schema_registry/_async/schema_registry_client.py

+
+    async def get_global_mode(self) -> str:
+        """
+        Get the current mode for Schema Reigstry at a global level.


Typo in docstring: 'Reigstry' should be 'Registry'.

Suggested change

Get the current mode for Schema Reigstry at a global level.

Get the current mode for Schema Registry at a global level.

sonarqube-confluent · 2025-08-21T22:29:22Z

Analysis Details

7 Issues

0 Bugs
0 Vulnerabilities
7 Code Smells

Coverage and Duplications

100.00% Coverage (66.60% Estimated after merge)
No duplication information (5.60% Estimated after merge)

Project ID: confluent-kafka-python

View in SonarQube

fangnx marked this pull request as ready for review August 18, 2025 20:37

Copilot AI review requested due to automatic review settings August 18, 2025 20:37

fangnx requested review from MSeal and a team as code owners August 18, 2025 20:37

fangnx changed the base branch from master to sr-endpoints-add-1 August 18, 2025 20:38

This comment was marked as outdated.

Sign in to view

airlock-confluentinc bot force-pushed the sr-endpoints-add-2 branch from ddf1826 to dc8d820 Compare August 18, 2025 20:40

This comment has been minimized.

Sign in to view

fangnx requested a review from rayokota August 18, 2025 21:08

This comment has been minimized.

Sign in to view

MSeal requested changes Aug 18, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

Base automatically changed from sr-endpoints-add-1 to master August 20, 2025 14:19

fangnx added 13 commits August 20, 2025 13:19

updaet

fa03877

update

4ff90b1

tests

d3674ff

fix integration tests

08e4b26

regenerate sync code + fix it

aa8604e

fix some unit tests broken from unasync

596477e

missing params and address feedback

24ea666

lint

a190cd6

revert breaking chamges

2358683

update

dcff38c

update

66f6882

update

bb05bb6

mode

7634f0d

fangnx added 6 commits August 20, 2025 13:35

mode

babd9bc

updaste

d61bee1

linter

d2e71da

fix typos

75400bd

update

710d390

add more

9b468a7

airlock-confluentinc bot force-pushed the sr-endpoints-add-2 branch from 7a24c92 to 9b468a7 Compare August 20, 2025 18:04

update

2c45803

This comment has been minimized.

Sign in to view

fix merge conflict errors

11ebe40

This comment has been minimized.

Sign in to view

fangnx requested a review from Copilot August 21, 2025 20:32

Copilot AI reviewed Aug 21, 2025

View reviewed changes

update

9c41f3b

		)
		return response['is_compatible'] # TODO: should it return entire response (including error messages)?

	\| GET \| /config/notfound \| 40401 \| Subject not found \|
	\| GET \| /config/notfound \| 40401 \| Subject not found (uses 'notfound' trigger) \|

	return response['is_compatible'] # TODO: should it return entire response (including error messages)?
	return response

	verbose (bool): Wehther to return detailed error messages.
	verbose (bool): Whether to return detailed error messages.

	Get the current mode for Schema Reigstry at a global level.
	Get the current mode for Schema Registry at a global level.

Add missing endpoints (/compatibility, /mode) to SchemaRegistryClient #2024

Are you sure you want to change the base?

Add missing endpoints (/compatibility, /mode) to SchemaRegistryClient #2024

Conversation

fangnx commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Checklist

References

Test & Review

Open questions / Follow-ups

Uh oh!

confluent-cla-assistant bot commented Aug 18, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

MSeal Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

fangnx Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

MSeal Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

fangnx Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqube-confluent bot commented Aug 21, 2025

Analysis Details

7 Issues

Coverage and Duplications

Uh oh!

Uh oh!

fangnx commented Aug 18, 2025 •

edited

Loading